Sync egglog subtrees for eggcc container benchmark#9
Merged
Conversation
6f0d26e3 Merge pull request #927 from oflatt/oflatt-container-proofs 85ba2626 small feedback fixes 4141a970 Merge pull request #935 from egraphs-good/nightly 5009aaee simplify map.rs e45cd547 address feedback from Saul 3960015c Merge pull request #885 from saulshanabrook/codex/split-scheduler-stale-match-cleanup bd1697a3 Tighten the Containers section of proof_encoding.md (#10) 5fca83b7 Add container support to the term/proof encoding 7f111370 nightly: prefer the rustup cargo shim over the system cargo (#942) 15888b87 nightly: install deps into a venv to satisfy PEP 668 (#939) f295052e scheduler: skip subsumed fresh matches d1418d20 Merge remote-tracking branch 'upstream/main' into codex/split-scheduler-stale-match-cleanup f99482a2 Merge pull request #936 from oflatt-claude/nightly-eval-live fe240e6b Render nightly benchmark report with eval-live 5bb42afc nightly 6c6f01d5 scheduler: simplify changelog entry c1420100 scheduler: reduce held validation churn ce4ef0b5 scheduler: validate only held matches f0d4822b scheduler: keep stale validation focused 1dedb505 scheduler: condense changelog entry 939339b1 scheduler: simplify stale match validation ed6aa6e9 scheduler: preserve existing match handling 8602f56e scheduler: split action-key dedup cleanup 89779541 scheduler: validate only held matches e24d1468 scheduler: clarify stale match validation eaf4ab4f Merge remote-tracking branch 'upstream/main' into codex/split-scheduler-stale-match-cleanup 0804bee1 scheduler: deduplicate residual match keys 2c55caff Merge remote-tracking branch 'upstream/main' into codex/split-scheduler-stale-match-cleanup 09b8c002 scheduler: document stale match validation 2c749b89 scheduler: clean stale matches git-subtree-dir: egglog git-subtree-split: 6f0d26e37241055a1230bdcdb33f0c5d3f01e935
…gcc-container-benchmark
5d24c481 Refresh eggcc proof baseline onto PR 927 core 19ed8666 Remove redundant eggcc helper fixture fcdaccba Add full eggcc proof fixture 6b37b6b3 Add eggcc proof baseline helpers git-subtree-dir: egglog-experimental git-subtree-split: 5d24c48197f796e130457fa2f0c10a60dfe258c7
…gcc-container-benchmark # Conflicts: # egglog-experimental/Cargo.toml # egglog-experimental/src/keep_best.rs # egglog-experimental/src/lib.rs # egglog-experimental/src/set_cost.rs # egglog-experimental/src/sugar/for.rs # egglog-experimental/src/table_stats.rs
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (71)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR adds container sort proof/rebuild support to egglog core (ValueRebuilder trait split, canonical term forms, proof encoding/checking for containers), new Maybe/Either sorts and merge primitives in egglog-experimental, hash index rebuild performance rework, a scheduler fix, and a nightly benchmark harness. ChangesCore container proofs and rebuild support
Egglog-experimental container primitives
Hash index performance and scheduler fix
Nightly benchmark harness
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant Typechecker
participant ProofState
participant EGraph
participant ContainerRebuild
Typechecker->>ProofState: record uf/proof_func/proof_constructors metadata
Typechecker->>ContainerRebuild: register_container_rebuild_from_spec(sort, spec)
EGraph->>ProofState: restore uf_parent/proof_names on Sort command
EGraph->>ContainerRebuild: invoke rebuild primitive on container value
ContainerRebuild-->>EGraph: rebuilt canonical container value / proof
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will not alter performance
Performance Changes
Comparing 
Footnotes |
oflatt
approved these changes
Jul 8, 2026
oflatt
left a comment
oflatt
left a comment
Collaborator
There was a problem hiding this comment.
Nice, I assume this just syncs so approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
egglog/core-relations/src/hash_index/mod.rs (1)
584-613: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a
debug_assertto guard the sorted invariant relied on byunsafeinget_subset.The Sparse subsets built here feed
SparseColumnIndex::get_subset, whoseunsafeblock assumes RowIds within each per-key range are non-decreasing (rows sorted by(Value, RowId)). Correctness today depends entirely on every caller pre-sortingpairs; a future caller that forgets would silently break that safety assumption. A cheap debug-only check documents and enforces the contract at the boundary.🛡️ Proposed defensive check
fn build_subsets_from_sorted(&mut self, pairs: &[(Value, RowId)]) { + debug_assert!( + pairs.windows(2).all(|w| w[0] <= w[1]), + "build_subsets_from_sorted requires pairs sorted by (Value, RowId); \ + get_subset relies on non-decreasing RowIds per key" + ); let mut i = 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@egglog/core-relations/src/hash_index/mod.rs` around lines 584 - 613, The sorted-order contract for build_subsets_from_sorted is only implied today, but SparseColumnIndex::get_subset’s unsafe path depends on it. Add a debug-only assertion at the start of build_subsets_from_sorted that verifies pairs are sorted by (Value, RowId) and contain no duplicate (Value, RowId) entries, so any future caller violating the invariant fails fast before reaching get_subset. Keep the check local to build_subsets_from_sorted and reference the existing precondition in its documentation.egglog/scripts/requirements.txt (1)
1-1: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin
eval-liveto a specific commit for reproducibility.An unpinned git dependency means any breaking change in
eval-livewill silently break the nightly benchmark. Pin to a commit hash and update it deliberately.🔒️ Proposed fix
-eval-live @ git+https://github.com/oflatt/eval-live.git +eval-live @ git+https://github.com/oflatt/eval-live.git@<commit-sha>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@egglog/scripts/requirements.txt` at line 1, The eval-live dependency is using an unpinned git reference, so update the requirements entry to use a specific commit hash instead of the floating branch head. Make this change in the requirements entry for eval-live in the dependency list, and keep the package name and git URL the same while adding the commit-specific pin for reproducible nightly runs.egglog/src/proofs/proof_container_rebuild.rs (1)
277-368: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReflexive anchor at the outer level looks duplicated by the caller.
rebuild_container_proof_rec's trailing anchor block (Lines 353-365) sets<CSort>Proof(rebuilt) = Trans(Sym(current), current)for every recursion level, including the top-level (sort == container_sort). Butproof_encoding.rs::rebuilding_rulesalready emits an equivalent action for the outer container column:(let proof_var (proof_prim ci)) (set (cproof rebuilt_var) (trans (sym proof_var) proof_var))
rebuilt_var/proof_varthere should equal this function'srebuilt/currentfor the outer call, so the same table row appears to get set twice per rebuild firing (once via rawinsertinside the primitive, once via the rule'sset). The primitive-side anchor is genuinely needed for nested containers (sincerebuilding_rulesonly handles the top column), but seems redundant at the outer level.
:merge oldand hash-consedsym/translikely make this harmless, but it's wasted work on a per-rebuild hot path and adds a second source of truth for the same invariant.♻️ Possible fix: only anchor at nested recursion levels
fn rebuild_container_proof_rec( state: &mut FullState, prim: &ContainerRebuildProof, sort: &ArcSort, value: Value, + is_top_level: bool, ) -> Option<(Value, Value)> { @@ - // Anchor a reflexive proof on the rebuilt value for future rebuilds. - if rebuilt != value { + // Anchor a reflexive proof on the rebuilt value for future rebuilds. + // The top-level anchor is already set by the caller's rebuild rule action. + if !is_top_level && rebuilt != value {Please confirm this is intentional (or a leftover from before the caller-side anchoring existed) before deciding whether to change it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@egglog/src/proofs/proof_container_rebuild.rs` around lines 277 - 368, The trailing reflexive anchor in rebuild_container_proof_rec duplicates the outer-level proof row already emitted by proof_encoding.rs::rebuilding_rules, so adjust the anchoring logic to skip inserting the cproof row for the top-level container while keeping it for nested container recursion. Keep the existing proof construction in rebuild_container_proof_rec, but gate the final sym/trans/cproof insert path so only recursive inner container rebuilds mint the anchor, using the existing symbols rebuild_container_proof_rec, cproof_action, sym_action, and trans_action to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@egglog-experimental/src/either.rs`:
- Around line 4-7: The import list in either.rs is missing Term, which is needed
by the validator closures that reference Term::App. Update the top-level egglog
import used near add_primitive_with_validator to include Term so the
Either-related validator code compiles without unresolved symbol errors.
In `@egglog-experimental/src/maybe.rs`:
- Around line 63-81: `MaybeSort::make_sort` currently panics when the sort
arguments don’t match the expected shape, so malformed input can crash
typechecking instead of returning a `TypeError`. Update the arity handling in
`make_sort` to validate the incoming `args` and return an appropriate error for
invalid container arity rather than panicking, and apply the same fix to
`EitherSort::make_sort` so both sort constructors fail gracefully on bad user
input.
In `@egglog/scripts/nightly_bench.py`:
- Around line 69-75: The proof exclusion list in nightly_bench.py has drifted
from the exclusions in tests/files.rs because eqsolve.egg is missing, so update
the PROOF_UNSUPPORTED_FILES source used by the proof benchmark setup to stay
aligned with the test harness. Prefer sharing one central list or helper for the
exclusions instead of duplicating the filenames, and make sure the code that
drives the nightly proof benchmarks references that shared source so future
additions stay in sync.
In `@egglog/src/ast/mod.rs`:
- Around line 1678-1688: The `replace_internal_symbol_with` path in `ast::mod`
is not updating the `container_rebuild` and `proof_constructors` fields when
building `GenericCommand::Sort`, so internal symbols can survive serialization
and parsing. Update the same `map_string_symbols`/`fun` transformation used for
`name`, `uf`, and `proof_func` to also map the symbol data inside
`container_rebuild` and `proof_constructors`, keeping the desugared sort command
round-trippable.
---
Nitpick comments:
In `@egglog/core-relations/src/hash_index/mod.rs`:
- Around line 584-613: The sorted-order contract for build_subsets_from_sorted
is only implied today, but SparseColumnIndex::get_subset’s unsafe path depends
on it. Add a debug-only assertion at the start of build_subsets_from_sorted that
verifies pairs are sorted by (Value, RowId) and contain no duplicate (Value,
RowId) entries, so any future caller violating the invariant fails fast before
reaching get_subset. Keep the check local to build_subsets_from_sorted and
reference the existing precondition in its documentation.
In `@egglog/scripts/requirements.txt`:
- Line 1: The eval-live dependency is using an unpinned git reference, so update
the requirements entry to use a specific commit hash instead of the floating
branch head. Make this change in the requirements entry for eval-live in the
dependency list, and keep the package name and git URL the same while adding the
commit-specific pin for reproducible nightly runs.
In `@egglog/src/proofs/proof_container_rebuild.rs`:
- Around line 277-368: The trailing reflexive anchor in
rebuild_container_proof_rec duplicates the outer-level proof row already emitted
by proof_encoding.rs::rebuilding_rules, so adjust the anchoring logic to skip
inserting the cproof row for the top-level container while keeping it for nested
container recursion. Keep the existing proof construction in
rebuild_container_proof_rec, but gate the final sym/trans/cproof insert path so
only recursive inner container rebuilds mint the anchor, using the existing
symbols rebuild_container_proof_rec, cproof_action, sym_action, and trans_action
to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71e2198d-e07d-4f49-96d1-eb6136cbff73
⛔ Files ignored due to path filters (7)
egglog-experimental/tests/fixtures/eggcc-2mm-pass1-merge-old.eggis excluded by!**/*.eggegglog/src/proofs/snapshots/egglog__proofs__proof_tests__tests__doc_example_add_function1.snapis excluded by!**/*.snapegglog/tests/container-proofs.eggis excluded by!**/*.eggegglog/tests/proofs/container-set-collapse.eggis excluded by!**/*.eggegglog/tests/snapshots/files__proof_unsupported_files.snapis excluded by!**/*.snapegglog/tests/snapshots/files__shared_snapshot_container_proofs.snapis excluded by!**/*.snapegglog/tests/snapshots/files__shared_snapshot_container_set_collapse.snapis excluded by!**/*.snap
📒 Files selected for processing (48)
egglog-experimental/src/container_primitives.rsegglog-experimental/src/either.rsegglog-experimental/src/lib.rsegglog-experimental/src/main.rsegglog-experimental/src/maybe.rsegglog-experimental/src/set_cost.rsegglog-experimental/tests/eggcc_2mm_proof.rsegglog-experimental/tests/files.rsegglog/.gitignoreegglog/CHANGELOG.mdegglog/Makefileegglog/core-relations/src/containers/mod.rsegglog/core-relations/src/containers/tests.rsegglog/core-relations/src/hash_index/mod.rsegglog/core-relations/src/hash_index/tests.rsegglog/core-relations/src/lib.rsegglog/core-relations/src/table_spec.rsegglog/core-relations/src/uf/mod.rsegglog/egglog-bridge/src/tests.rsegglog/scripts/nightly_bench.pyegglog/scripts/requirements.txtegglog/src/ast/desugar.rsegglog/src/ast/mod.rsegglog/src/ast/parse.rsegglog/src/lib.rsegglog/src/prelude.rsegglog/src/proofs/mod.rsegglog/src/proofs/proof_checker.rsegglog/src/proofs/proof_container_rebuild.rsegglog/src/proofs/proof_encoding.mdegglog/src/proofs/proof_encoding.rsegglog/src/proofs/proof_encoding_helpers.rsegglog/src/proofs/proof_extraction.rsegglog/src/proofs/proof_format.rsegglog/src/proofs/proof_normal_form.rsegglog/src/proofs/proof_simplification.rsegglog/src/proofs/proof_tests.rsegglog/src/scheduler.rsegglog/src/sort/fn.rsegglog/src/sort/map.rsegglog/src/sort/mod.rsegglog/src/sort/multiset.rsegglog/src/sort/pair.rsegglog/src/sort/set.rsegglog/src/sort/vec.rsegglog/src/termdag.rsegglog/src/typechecking.rsegglog/tests/container_rebuild.rs
Comment on lines
+4
to
+7
| use egglog::{ | ||
| ArcSort, ContainerValue, EGraph, TermDag, TermId, TypeError, TypeInfo, Value, | ||
| add_primitive_with_validator, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm Term is used but not imported in either.rs
echo "--- imports ---"; rg -nP 'use egglog::\{' -A3 egglog-experimental/src/either.rs
echo "--- Term usages ---"; rg -n 'Term::' egglog-experimental/src/either.rsRepository: saulshanabrook/egglog-encoding
Length of output: 552
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure and inspect the surrounding context for any local `Term` definition or wildcard import.
ast-grep outline egglog-experimental/src/either.rs --view expanded || true
echo "--- nearby source around Term usages ---"
sed -n '1,260p' egglog-experimental/src/either.rs | nl -ba | sed -n '1,240p'Repository: saulshanabrook/egglog-encoding
Length of output: 1420
Add Term to the import list. Term::App is used in the validator closures below, but egglog::Term is not imported here, so this file won’t compile.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@egglog-experimental/src/either.rs` around lines 4 - 7, The import list in
either.rs is missing Term, which is needed by the validator closures that
reference Term::App. Update the top-level egglog import used near
add_primitive_with_validator to include Term so the Either-related validator
code compiles without unresolved symbol errors.
Comment on lines
+63
to
+81
| fn make_sort( | ||
| typeinfo: &mut TypeInfo, | ||
| name: String, | ||
| args: &[Expr], | ||
| ) -> Result<ArcSort, TypeError> { | ||
| if let [Expr::Var(span, element)] = args { | ||
| let element = typeinfo | ||
| .get_sort_by_name(element) | ||
| .ok_or(TypeError::UndefinedSort(element.clone(), span.clone()))?; | ||
|
|
||
| Ok(Self { | ||
| name, | ||
| element: element.clone(), | ||
| } | ||
| .to_arcsort()) | ||
| } else { | ||
| panic!("Maybe sort requires exactly one argument") | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether presort arity is validated before make_sort is called, and how other presorts handle wrong arity.
rg -nP 'fn make_sort' -A15 egglog/src/sort/ egglog-experimental/src/
rg -nP 'add_presort|make_sort\s*\(' -n egglog/src/typechecking.rs egglog/src/sort/mod.rsRepository: saulshanabrook/egglog-encoding
Length of output: 9241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the presort registration path and the relevant make_sort implementations.
sed -n '620,760p' egglog/src/typechecking.rs
echo '---'
sed -n '1,220p' egglog/src/sort/mod.rs
echo '---'
sed -n '1,180p' egglog-experimental/src/maybe.rs
echo '---'
sed -n '1,180p' egglog-experimental/src/either.rs
echo '---'
sed -n '1,180p' egglog/src/sort/vec.rs
echo '---'
sed -n '1,180p' egglog/src/sort/set.rs
echo '---'
sed -n '1,180p' egglog/src/sort/multiset.rs
echo '---'
sed -n '1,180p' egglog/src/sort/map.rs
echo '---'
sed -n '1,180p' egglog/src/sort/pair.rs
echo '---'
sed -n '1,180p' egglog/src/sort/fn.rsRepository: saulshanabrook/egglog-encoding
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where presort constructors are invoked and whether arg arity is validated beforehand.
rg -n "mksorts|make_sort\(|Presort" egglog/src egglog-experimental/src
echo '---'
sed -n '1,260p' egglog/src/typechecking.rsRepository: saulshanabrook/egglog-encoding
Length of output: 10918
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the sort declaration parsing / resolution path to see whether arg arity is constrained upstream.
sed -n '220,280p' egglog/src/typechecking.rs
echo '---'
rg -n "declare_sort\(" egglog/src egglog-experimental/src
echo '---'
rg -n "presort_and_args|Sort.*args|sort .*\\(" egglog/src egglog-ast/src egglog-experimental/srcRepository: saulshanabrook/egglog-encoding
Length of output: 15139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect parser handling of sort/presort arguments and any arity checks.
sed -n '260,420p' egglog/src/ast/parse.rs
echo '---'
sed -n '1000,1060p' egglog/src/ast/mod.rsRepository: saulshanabrook/egglog-encoding
Length of output: 10765
Return TypeError instead of panicking on invalid sort arity.
MaybeSort::make_sort (and EitherSort::make_sort) panic on malformed (sort ...) input, and container args are forwarded straight through, so bad user input can crash typechecking instead of producing a TypeError. Handle the wrong-arity case here and return an error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@egglog-experimental/src/maybe.rs` around lines 63 - 81,
`MaybeSort::make_sort` currently panics when the sort arguments don’t match the
expected shape, so malformed input can crash typechecking instead of returning a
`TypeError`. Update the arity handling in `make_sort` to validate the incoming
`args` and return an appropriate error for invalid container arity rather than
panicking, and apply the same fix to `EitherSort::make_sort` so both sort
constructors fail gracefully on bad user input.
Comment on lines
+1678
to
+1688
| container_rebuild, | ||
| proof_constructors, | ||
| unionable, | ||
| } => GenericCommand::Sort { | ||
| span, | ||
| name: fun(name), | ||
| presort_and_args, | ||
| uf: uf.map(&mut *fun), | ||
| uf: uf.map(|(ctor, index)| (fun(ctor), index.map(&mut *fun))), | ||
| proof_func: proof_func.map(&mut *fun), | ||
| container_rebuild, | ||
| proof_constructors, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect sanitize_internal_symbols and how it uses map_string_symbols,
# and whether rebuild-prim / proof-constructor names are reserved/renamed.
rg -nP -C4 'fn sanitize_internal_symbols' egglog/src
rg -nP -C3 'map_string_symbols' egglog/src
# Where are internal_rebuild_prim / proof constructor names generated?
rg -nP -C3 'internal_rebuild_prim|internal_rebuild_proof_prim' egglog/src
rg -nP -C3 'congr_constructor|container_normalize_constructor|eq_trans_constructor|eq_sym_constructor' egglog/srcRepository: saulshanabrook/egglog-encoding
Length of output: 26720
🏁 Script executed:
#!/bin/bash
sed -n '1660,1705p' egglog/src/ast/mod.rs
printf '\n---\n'
sed -n '2136,2170p' egglog/src/ast/mod.rs
printf '\n---\n'
sed -n '1,120p' egglog/src/proofs/proof_container_rebuild.rs
printf '\n---\n'
sed -n '1,120p' egglog/src/proofs/proof_format.rsRepository: saulshanabrook/egglog-encoding
Length of output: 13412
🏁 Script executed:
#!/bin/bash
rg -n -C3 'INTERNAL_SYMBOL_PREFIX|sanitize_internal_symbols|replace_internal_symbol_with|ContainerRebuildSpec|proof_constructors|container_rebuild' egglog/src
printf '\n---\n'
sed -n '1,120p' egglog/src/ast/mod.rs
printf '\n---\n'
sed -n '440,500p' egglog/src/typechecking.rs
printf '\n---\n'
sed -n '1600,1645p' egglog/src/lib.rsRepository: saulshanabrook/egglog-encoding
Length of output: 50386
Map container_rebuild and proof_constructors here too
replace_internal_symbol_with runs through map_string_symbols, and these annotations are serialized on (sort ...) and read back during parse/typecheck. Leaving them untouched can preserve stale internal names and break round-tripping of the desugared program.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@egglog/src/ast/mod.rs` around lines 1678 - 1688, The
`replace_internal_symbol_with` path in `ast::mod` is not updating the
`container_rebuild` and `proof_constructors` fields when building
`GenericCommand::Sort`, so internal symbols can survive serialization and
parsing. Update the same `map_string_symbols`/`fun` transformation used for
`name`, `uf`, and `proof_func` to also map the symbol data inside
`container_rebuild` and `proof_constructors`, keeping the desugared sort command
round-trippable.
Stacked on the subtree-sync branch. This preserves the final reviewed tree from codex/eggcc-container-benchmark while separating local benchmark, proof, CI, and review-fix changes from the subtree merge commits. The hash index ordering fix from egraphs-good/egglog#914 lives in the subtree-sync branch.
…hmark-updates Add eggcc container benchmark and proof coverage
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is the subtree-sync base for the eggcc container benchmark work.
It contains:
egglog/from upstreamegraphs-good/egglogegglog-experimental/from upstreamegraphs-good/egglog-experimentalPR 56egglog/core-relations/src/hash_indexordering fix from Fix multi-column ColumnIndex rebuild ordering via per-column radix sort + merge egraphs-good/egglog#914The local benchmark, proof snapshot, CI, and review-fix changes are intentionally kept out of this PR and stacked in #10.
Review Notes
Review this as imported upstream/subtree work. The follow-up PR should be reviewed against this branch so the local changes are isolated from the subtree and PR 914 diffs.
Summary by CodeRabbit
New Features
Bug Fixes
Tests